Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add viewport meta #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add viewport meta #1

wants to merge 1 commit into from

Conversation

srsgores
Copy link

@srsgores srsgores commented Nov 4, 2013

Update requirejs
Add .idea to .gitignore

Update requirejs
Add .idea to .gitignore
@jescalan
Copy link

jescalan commented Nov 4, 2013

Hey Sean,

Thanks for taking the time to drop all these PRs in here, this is awesome. I'm entirely good with these changes, just a couple points:

  • I'm not sure I want to start adding text-editor-specific config files to the default .gitignore, there can be a lot of these and it seems like if your text editor generates files it should be up to you to add them. For example, in rails' gitignore, while they do have a fairly lengthy list of files in there, they are all rails specific rather than external dev tool specific.
  • I like having the viewport in there, but I'm not sure I'd want it as the default, as this assumes that your site is mobile compatible from the start - maybe it should be in there but commented out at first, so it would be easy to comment it in once you have started on the responsiveness? The only reason I mention this is because it's better to have a non-responsive site available on mobile but require zooming than it is to have the viewport set and everything look effed, if someone did not make their site responsive and forgot about the tag in there.

/cc @slang800 @samccone @declandewet for comment

@zspecza
Copy link

zspecza commented Nov 4, 2013

I like the commented out idea. Though if we were to add it as a default we might as well go the full way of including the HTML5 boilerplate...

@jescalan
Copy link

jescalan commented Nov 4, 2013

I'd be happy to support an html5 boilerplate template as well if someone wants to contribute that. I feel like the point of the templates originally was to allow anyone to make their own template that's exactly how they want it and set that as the default if they are not totally into our defaults. The min template is my preferred setup 😄

@zspecza
Copy link

zspecza commented Nov 4, 2013

I was just thinking that to be honest. And that's actually what I did. I use a custom template that gives me selectivizr.js, modernizr.js, boxsizing.htc and respond.js polyfills.

@jescalan
Copy link

jescalan commented Nov 4, 2013

Well let's see what you got - post it up! If people like it and you want we could move it to this org and ship with it by default.

@zspecza
Copy link

zspecza commented Nov 4, 2013

Well that's literally all it does at the moment, I just use those so often since discovering Jeet. Maybe I'll upload it some time.

@srsgores
Copy link
Author

@Jenius, what's the status on this PR?

I still think that including the meta tag makes sense, given most apps today are mobile-friendly. I guess I'm still fine with commenting it out, but my worry is that it might mislead the novice developer into thinking it's not a good idea. Maybe if we prefixed the comment with a line like:

<! -- Uncomment this block if your site is mobile-friendly -->

Then it would be clear.

@declandewet, as per your polyfill libraries, I think those should be left out, maybe for a legacy-support template.

@jescalan
Copy link

I left a detailed comment on it a number of months ago and it looks like you never responded. I have about the same view still - I'd like for the additions to the gitignore to be removed and for the mobile tag to be commented by default. I think the idea you just posted with the clarifying comment makes sense : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants